Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

respect escaped quotes in the valueRxString regex #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

beniwohli
Copy link

This resolves #2. The regex proposed in #2 (comment) turned out to not work correctly, and it was extremely slow (the test suite never finished). Instead, I adapted the regex from http://stackoverflow.com/a/10786066/45691. Here's the regex on regexdoc: https://regex101.com/r/wN6qS3/1.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@beniwohli
Copy link
Author

Is there any interest in this PR? I'll gladly spend the time to fix the merge conflicts that have accrued since opening it, but only if there's interest in merging it.

@jordansissel
Copy link
Contributor

Howdy! Sorry for the delays.

I like the idea as proposed. I have a question about the behavior, though. It feels like the escaped characters should be interpreted before storing in the event. For example, for a kv input of this:

foo="hello \"world\""

The resulting value for the foo field should be hello "world" (no backslashes). The way I read your tests, it seems like the backslashes are kept. Thoughts?

@beniwohli
Copy link
Author

Thanks for the feedback. Yes, you're right, the quoting back slashes shouldn't end up in the resulting string. I'll see if I can tweak the regex (it's been a while since I fully understood that regex, though :D )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kv filter: support escaped quotes
4 participants